See the review comment for full details.

Note: The following inline comments could not be posted on the diff (GitHub returned 422) and are included here instead:

• hack/upload-traces.sh (file-level): Line 74 · [medium] stale-reference

The script still validates that CONFIG_TEMPLATE (the static YAML file upload-traces-otelcol-config.yaml) exists on disk, but the generated RUNTIME_CONFIG is now used instead. The CONFIG_TEMPLATE variable (line 5) and its validation are dead code that will produce a misleading error if the now-unused template file is later removed.

Suggested fix: Remove the CONFIG_TEMPLATE variable (line 5) and its existence check (lines 74-75), or remove only the validation if the template is kept for reference.

• hack/upload-traces.sh:127: [medium] YAML-injection

Header keys and values are written into the generated YAML config with insufficient sanitization. Keys have no sanitization at all; values only escape double quotes but not embedded newlines (which break out of YAML double-quoted strings) or YAML backslash escape sequences. The OTEL_EXPORTER_OTLP_HEADERS env var could be set by CI automation, making this a viable injection path in shared environments.

Suggested fix: Validate keys match [a-zA-Z0-9_-]+ at parse time, strip newline characters from values, and consider single-quoted YAML scalars for values (which have no escape sequences).

• hack/upload-traces.sh:123: [low] edge-case

Header values containing commas are incorrectly split due to the comma-separated join/split approach. This is a pre-existing constraint inherited from the OTEL_EXPORTER_OTLP_HEADERS comma-separated format, not a regression introduced by this PR.

Automated multi-model review pass (2 independent reviewers completed; a third did not finish in time). Findings below are newly surfaced against the current head commit — the previously-flagged dead CONFIG_TEMPLATE reference and the original YAML-escaping gaps (missing key validation, raw double-quote-only escaping) are already fixed as of the latest commit and are not re-flagged here. A previously-discussed header-value comma-splitting limitation (inherited from the OTEL_EXPORTER_OTLP_HEADERS env-var convention, not a regression from this PR) is likewise not re-flagged. Posting 1 HIGH and 2 MEDIUM findings.

Recheck pass against head 2843acf6 (following up on the prior review round at an earlier commit).

Verified fixed:

• HIGH (cleartext header/secret printing to stdout): confirmed removed. The echo "headers: $MERGED_HEADERS" / cat "$RUNTIME_CONFIG" dump is gone; the script now prints only a redacted header count.
• MEDIUM ($ escaping gap): confirmed fixed. value="${value//\$/\$\$}" was added, correctly neutralizing the Collector's ${env:...} expansion syntax.

Still open in substance (threads already marked resolved on GitHub, so not re-flagged inline below, but noting for visibility):

• MEDIUM premature-decision (otelcol-contrib header-reading behavior asserted as fact): the code comment is unchanged and no end-to-end verification was added to the PR description; the thread's resolution rests on a reply rather than a code/doc change.
• MEDIUM (non-ASCII line-break escaping): only ASCII \n/\r are stripped; Unicode line-separator characters are still unhandled.

New findings surfaced during this recheck are below.

Recheck round 2

Re-reviewed at head 6848df27 (previous recheck was against 2843acf6). Dispatched a 4-agent review squad (independent Claude, Grok, and Codex-backed reviewers) against the current diff; findings below are the deduped, independently cross-verified survivors — each was confirmed by at least 3 of 4 agents and/or reproduced empirically against the real confmap library rather than inferred from reading code alone.

Confirmed resolved since the last recheck:

• Header secret values no longer persist in cleartext in the generated config file — only header keys and internal REPLAY_HEADER_N variable names ever touch disk now.
• Duplicate header keys are deduplicated with deterministic last-wins semantics (--header overrides OTEL_EXPORTER_OTLP_HEADERS).
• The mktemp template no longer has a suffix after XXXXXX, so it randomizes correctly on BSD/macOS mktemp (verified directly — the old template returned a literal, non-randomized path on the first call and hard-failed on every call after; the new one doesn't).
• The original stdout cleartext leak remains fixed (prints a redacted count only).

Still open / newly found (see inline comments below) — unresolving the premature-decision thread, since the substance persists under the reworded comment:

• The otelcol-contrib header-reading behavior comment now cites a specific version and method, but is still an unreproducible, unbacked assertion — the PR's own Testing checklist wasn't updated and the tool version pin is still floor-only.
• The fix that moved header values out of the YAML file text (into per-header env-var indirection) closed the literal-embedding escaping gap but opened a more serious one: the Collector's own config resolver recursively re-expands ${env:...}/${file:...}-shaped text found inside an already-resolved value, so an unsanitized --header/--endpoint value can pivot to exfiltrate an unrelated environment variable or local file. Verified empirically against the real confmap library.
• A new bash-4-only construct in this commit hard-crashes the entire header feature on stock macOS bash.

Recheck round 3

Re-reviewed at head afacbac2 (previous recheck was against 6848df27). Dispatched a 4-agent review squad (independent Claude, Grok, and Codex-backed reviewers) against the current diff, then independently verified the security-relevant claims myself against the real go.opentelemetry.io/collector/confmap source and by direct reproduction.

Verified fixed:

• CRITICAL (confmap recursive re-expansion): genuinely closed. escape_confmap() (line 107) doubles every literal $ to $$ before REPLAY_INCLUDE, REPLAY_ENDPOINT, and every header value are exported. Verified against confmap's actual findURI escape-detection logic (an odd run of $ immediately before ${ is required to trigger expansion; doubling a value's dollar signs always produces an even count, which is left as inert literal text) and against the project's own e2e fixture (confmap/internal/e2e/testdata/expand-escaped-env.yaml), which covers exactly this $${env:...} shape and asserts it resolves to the literal string, never the referenced variable. The >=0.120.0 floor this script requires postdates the $$-escaping fix (landed 0.105.0, stable 0.107.0, unconditional since 0.109.0), so there's no version gap in the supported range.
• HIGH (declare -A bash 3.2 crash): genuinely closed. No associative arrays remain anywhere in the file; dedup now uses plain indexed arrays with a linear scan, which is bash 3.0+-safe. Confirmed no other bash-4-only syntax was introduced, and confirmed the script runs end-to-end under real stock bash 3.2.57.

Not re-flagged (raised twice, maintainer has explicitly declined both — noting for visibility, not re-litigating): the unverified otelcol-contrib behavior comment, and the bearer-token-shaped usage example.

New findings below — surfaced while tracing the secret data flow end-to-end hunting for a possible 4th regression, independently reproduced, not duplicates of anything raised in prior rounds.